Skip to content

Adding schema registry, validator and generator for RF#279

Draft
TeresiaOlsson wants to merge 13 commits into
mainfrom
schema-registry-rf
Draft

Adding schema registry, validator and generator for RF#279
TeresiaOlsson wants to merge 13 commits into
mainfrom
schema-registry-rf

Conversation

@TeresiaOlsson

Copy link
Copy Markdown
Contributor

Adding the schema registry, validator and generator for the RF subpackage as a first step to do the refactoring while implementing required legacy handling to not break the code.

@TeresiaOlsson

Copy link
Copy Markdown
Contributor Author

@JeanLucPons and @gupichon while I work on rebasing on main and adding the final parts to be able to register the schema in the registry, can you maybe take a look at my suggestion for how to add the validation at object creation?

I thought it will be too confusing for the users to have one decorator which registers the schema + dynamically generates it + adds validation at object creation since it's too many thing happening at once. And overwriting the constructor in a decorator didn't feel great.

So instead I decided to try to add the validation at object creation using inheritance. There is now a metaclass ValidationMeta which implements the validation and two subclasses DynamicValidation and StaticValidation which dynamically generates the model to validate against, alternatively allows to add a model directly to the class in case the dynamic validation isn't sufficient and you want to use some more advanced feature from pydantic. The idea is if you want your class to validate at object creation you inherit from one of those two classes. The example of the usage is so far only in RFTransmitter.

What do you think about this?

@JeanLucPons

Copy link
Copy Markdown
Contributor

Adding an extra validation class is fine with me.
PYAMLCLASS is still needed ?

@TeresiaOlsson

Copy link
Copy Markdown
Contributor Author

I plan to also remove PYAMLCLASS. I just haven't dared to do it yet. Burnt by my last attempt I now try to change in as small steps as possible, run the tests, rebase based on current main in case Github says there is a merge conflict and then change the next small thing ;) Hopefully by the time the PR is finished that will mean an easier review.

@JeanLucPons

Copy link
Copy Markdown
Contributor
    obj = type(self)(
            name=self.name,
            cavities=self.cavities,
            voltage=self.voltage_str,
            phase=self.phase_str,
            harmonic=self.harmonic,
            distribution=self.distribution,
            lattice_names=self.lattice_names,
            description=self.description,
        )

To avoid code duplication you can replace the above code by:

  obj = copy.copy(self)

May be, I would be better to have phase_name and voltage_name rather that ..._str

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants